[SANDBOX-1701] chore: upgrade to OCP 4.21 dependencies#522
Conversation
Update openshift/api and openshift/library-go to release-4.21, k8s.io deps to v0.34.3, controller-runtime to v0.22.5. Adapt test FakeClient to work with real SSA support: - Remove Apply→Merge workaround (SSA is now natively supported) - Handle generation bump for SSA-created objects - Work around controller-runtime fake client bug where Update discards caller-set managedFields (needed for csaupgrade migration) - Add NewFakeClientWithManagedFields for tests that verify SSA managed fields behavior - Fix SSA migration test to provide APIVersion in ManagedFieldsEntry and use a non-empty Service spec for proper field tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated Go module dependency versions (Kubernetes, OpenShift, and various libraries), added a module replace directive, and expanded an inline SSA-compatibility comment in the test client’s Patch method. No exported API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 126-127: The go.mod contains a replace directive "replace
github.com/codeready-toolchain/api => github.com/metlos/api
v0.0.0-20260311111036-f94bc6f41690" that points to a personal fork; either
remove this replace or replace it with the official module, and if keeping it
temporarily, add an inline comment and a linked issue/PR that documents why the
fork is required and the plan to revert (e.g., add a TODO with issue/PR number
and an expiration rationale next to the replace directive), and update the
commit message to explain the temporary nature and timeline for removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef7f35c2-5aec-455e-9990-5b5b276c6e61
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modpkg/client/ssa_client_test.gopkg/test/client.go
|
|
||
| replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260311111036-f94bc6f41690 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's an open PR in the codeready-toolchain/api repo that this fork might be based on
echo "Checking the fork commit referenced in the replace directive..."
echo "Fork: github.com/metlos/api @ v0.0.0-20260311111036-f94bc6f41690"
echo ""
echo "This appears to be a personal fork. Verify that:"
echo "1. The required API changes are being upstreamed"
echo "2. This replace directive will be removed before merge or shortly after"
echo ""
# Check if the main API repo has recent activity that might include these changes
gh api repos/codeready-toolchain/api/pulls --jq '.[0:5] | .[] | {number, title, state, user: .user.login}' 2>/dev/null || echo "Could not fetch PR info"Repository: codeready-toolchain/toolchain-common
Length of output: 516
🏁 Script executed:
# Check the actual content of go.mod at the specified lines
cat -n go.mod | sed -n '120,135p'Repository: codeready-toolchain/toolchain-common
Length of output: 502
🏁 Script executed:
# Check git history for this change
git log --oneline -20 -- go.modRepository: codeready-toolchain/toolchain-common
Length of output: 127
🏁 Script executed:
# Look for any recent commits that added this replace directive
git log -p -S "metlos/api" -- go.mod | head -100Repository: codeready-toolchain/toolchain-common
Length of output: 62
🏁 Script executed:
# Get the full commit message and diff context
git show bbfe574 --stat | head -50Repository: codeready-toolchain/toolchain-common
Length of output: 2799
🏁 Script executed:
# Check if there's any branch information or PR context
git log bbfe574 -1 --format=fullerRepository: codeready-toolchain/toolchain-common
Length of output: 1104
🏁 Script executed:
# Check if there are any CHANGELOG, PR templates, or issue references
git log bbfe574 -1 --format=%BRepository: codeready-toolchain/toolchain-common
Length of output: 814
🏁 Script executed:
# Check if the replace directive is still in the current HEAD
git rev-parse HEADRepository: codeready-toolchain/toolchain-common
Length of output: 120
🏁 Script executed:
# Verify this is the actual current state
cat go.mod | grep -A 2 -B 2 "replace github.com/codeready-toolchain/api"Repository: codeready-toolchain/toolchain-common
Length of output: 185
🏁 Script executed:
# Check if there are any recent commits after this that might address it
git log --oneline -10 -- go.modRepository: codeready-toolchain/toolchain-common
Length of output: 127
Remove or document the replace directive for the personal fork of codeready-toolchain/api.
The replace directive points to a personal fork (github.com/metlos/api) instead of the official repository. While this was included in the OCP 4.21 dependency upgrade commit, there is no explanation in the commit message for why this fork is necessary. This creates maintenance and security risks:
- Personal forks can be deleted, archived, or diverge from the official repo
- It creates an undocumented dependency that complicates dependency management
- It may cause resolution issues in CI/CD pipelines or for downstream consumers
Either document the temporary nature of this fork and the plan to remove it, or replace it with the official github.com/codeready-toolchain/api repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 126 - 127, The go.mod contains a replace directive
"replace github.com/codeready-toolchain/api => github.com/metlos/api
v0.0.0-20260311111036-f94bc6f41690" that points to a personal fork; either
remove this replace or replace it with the official module, and if keeping it
temporarily, add an inline comment and a linked issue/PR that documents why the
fork is required and the plan to revert (e.g., add a TODO with issue/PR number
and an expiration rationale next to the replace directive), and update the
commit message to explain the temporary nature and timeline for removal.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
126-127:⚠️ Potential issue | 🟠 MajorThe personal fork replace directive persists.
The replace directive still points to a personal fork (
github.com/metlos/api), though updated to a newer commit. The concerns raised in the previous review comment regarding maintainability, security, and dependency resolution remain applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 126 - 127, Remove the personal-fork replace directive that references "github.com/metlos/api" in go.mod and instead point the module to the official upstream module (restore or add a versioned requirement for "github.com/codeready-toolchain/api" with a proper semantic version or a vetted commit pseudo-version); if a forked module is absolutely required for a temporary fix, replace it with an organizationally-approved fork and document the reason in a comment, and ensure the replace line referencing "github.com/metlos/api" is deleted or replaced accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Around line 126-127: Remove the personal-fork replace directive that
references "github.com/metlos/api" in go.mod and instead point the module to the
official upstream module (restore or add a versioned requirement for
"github.com/codeready-toolchain/api" with a proper semantic version or a vetted
commit pseudo-version); if a forked module is absolutely required for a
temporary fix, replace it with an organizationally-approved fork and document
the reason in a comment, and ensure the replace line referencing
"github.com/metlos/api" is deleted or replaced accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc918ed0-04a8-425d-8a72-c1b9acbe4598
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
| } | ||
|
|
||
| return cl.Client.Update(ctx, obj, opts...) | ||
| // Work around a controller-runtime fake client bug where Update unconditionally |
There was a problem hiding this comment.
interesting, is there a ling to an issue we could track?
and the associated changes to the SSA handling in the test client.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
127-127:⚠️ Potential issue | 🔴 CriticalRemove the forbidden module replacement before merge (CI blocker).
Line 127 replaces
github.com/codeready-toolchain/apiwith a personal fork, andci-check-gomodis already failing on this exact rule. This blocks merge and introduces avoidable supply-chain/maintenance risk.Proposed fix
-replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260327105121-b79d7632f264#!/bin/bash # Verify the blocker is removed from go.mod rg -n '^replace\s+github\.com/codeready-toolchain/api\s*=>' go.mod || true # Show effective replacement (should print nothing after fix) go list -m all 2>/dev/null | rg -n 'github.com/codeready-toolchain/api.*=>'As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 127, Remove the forbidden replace directive that swaps github.com/codeready-toolchain/api to the personal fork (the replace statement currently added in go.mod); revert to the upstream module by deleting that replace line and, if a specific version is required, add an approved semantic version for github.com/codeready-toolchain/api instead of a fork so ci-check-gomod passes and supply-chain risk is removed (this touches the replace directive for github.com/codeready-toolchain/api in go.mod and will resolve the ci-check-gomod failure).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 127: Remove the forbidden replace directive that swaps
github.com/codeready-toolchain/api to the personal fork (the replace statement
currently added in go.mod); revert to the upstream module by deleting that
replace line and, if a specific version is required, add an approved semantic
version for github.com/codeready-toolchain/api instead of a fork so
ci-check-gomod passes and supply-chain risk is removed (this touches the replace
directive for github.com/codeready-toolchain/api in go.mod and will resolve the
ci-check-gomod failure).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e71aade4-2340-457d-93a6-5bef26176a33
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modpkg/test/client.go
✅ Files skipped from review due to trivial changes (1)
- pkg/test/client.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
go.mod (1)
128-128:⚠️ Potential issue | 🟠 MajorReplace to personal fork remains a supply-chain/maintenance risk.
Line 128 still replaces
github.com/codeready-toolchain/apiwithgithub.com/metlos/api. If this is temporary, add an inline TODO with linked tracking issue/PR and removal plan; otherwise switch back to the official module before merge.#!/bin/bash # Check replace directive and whether it is documented inline cat -n go.mod | sed -n '122,132p' rg -n 'replace github.com/codeready-toolchain/api' go.mod rg -n 'TODO|FIXME|tracking issue|remove.*replace|metlos/api' go.modAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 128, The go.mod replace directive swapping github.com/codeready-toolchain/api to github.com/metlos/api is a supply-chain/maintenance risk; either revert the replace to the official module or, if it must remain temporarily, annotate the replace line in go.mod with an inline TODO that references a tracking issue/PR number and a clear removal plan and deadline. Update the replace directive comment to include the tracking issue/PR link and expected removal date, and ensure CI/PR description documents why the fork is used; if reverting, remove the replace directive and run go mod tidy to restore the canonical dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 22-25: Update the controller-runtime module version in go.mod:
change the existing declaration sigs.k8s.io/controller-runtime v0.21.0 to
v0.22.0 so it matches the Kubernetes modules (k8s.io/api, k8s.io/apimachinery,
k8s.io/client-go at v0.34.1); run go mod tidy afterwards to resolve transitive
dependencies and rebuild to ensure there are no API incompatibilities in your
controller code.
---
Duplicate comments:
In `@go.mod`:
- Line 128: The go.mod replace directive swapping
github.com/codeready-toolchain/api to github.com/metlos/api is a
supply-chain/maintenance risk; either revert the replace to the official module
or, if it must remain temporarily, annotate the replace line in go.mod with an
inline TODO that references a tracking issue/PR number and a clear removal plan
and deadline. Update the replace directive comment to include the tracking
issue/PR link and expected removal date, and ensure CI/PR description documents
why the fork is used; if reverting, remove the replace directive and run go mod
tidy to restore the canonical dependency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 223b70c2-90d3-4bab-8917-75818e93c58c
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
| k8s.io/api v0.34.1 | ||
| k8s.io/apimachinery v0.34.1 | ||
| k8s.io/client-go v0.34.1 | ||
| sigs.k8s.io/controller-runtime v0.21.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current version skew in go.mod
rg -n '^\s*(k8s\.io/(api|apimachinery|client-go|kubectl|cli-runtime|apiextensions-apiserver)|sigs\.k8s\.io/controller-runtime)\s+v' go.modRepository: codeready-toolchain/toolchain-common
Length of output: 1821
🌐 Web query:
What Kubernetes minor version compatibility is documented for sigs.k8s.io/controller-runtime v0.21.0 and v0.22.0?
💡 Result:
- controller-runtime v0.21.0: documented as aligned with k8s.io/client-go v0.33.x, which corresponds to Kubernetes v1.33.x. [1][2]
- controller-runtime v0.22.0: documented as aligned with k8s.io/client-go v0.34.x, which corresponds to Kubernetes v1.34.x. [1][2]
Sources:
[1] controller-runtime repo README “Compatibility” table (CR v0.21 → client-go v0.33; CR v0.22 → client-go v0.34) (github.com)
[2] client-go INSTALL.md: v0.x.y tags correspond to Kubernetes v1.x.y (chromium.googlesource.com)
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime
- 2: https://chromium.googlesource.com/external/github.com/kubernetes/client-go/%2Bshow/af4295f5013b55c45b05f1e7850f7fa9c4dbfdb4/INSTALL.md
Upgrade sigs.k8s.io/controller-runtime to v0.22.0 to match Kubernetes v0.34.1 modules.
Line 25 keeps sigs.k8s.io/controller-runtime v0.21.0 while Lines 22-24 use Kubernetes modules at v0.34.1. According to controller-runtime's official compatibility matrix, v0.21.0 is designed for k8s v0.33.x, while v0.22.0 pairs with v0.34.x. This version skew causes API/type incompatibilities in controller code and violates maintainability requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 22 - 25, Update the controller-runtime module version in
go.mod: change the existing declaration sigs.k8s.io/controller-runtime v0.21.0
to v0.22.0 so it matches the Kubernetes modules (k8s.io/api,
k8s.io/apimachinery, k8s.io/client-go at v0.34.1); run go mod tidy afterwards to
resolve transitive dependencies and rebuild to ensure there are no API
incompatibilities in your controller code.
… for openshift api 4.21 + codeready-toolchain/api
|
|
Postponed until operator-sdk is upgradable, too. |
3 participants
Update openshift/api and openshift/library-go to release-4.21, k8s.io deps to v0.34.1, controller-runtime to v0.22.0.
Adapt test FakeClient to work with real SSA support:
Assisted-by: Claude Code
Summary by CodeRabbit